Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

v2.0 #323

Merged
merged 24 commits into from
Jan 20, 2021
Merged

v2.0 #323

merged 24 commits into from
Jan 20, 2021

Conversation

znicholls
Copy link
Collaborator

@znicholls znicholls commented Jan 17, 2021 •
edited
Loading

Pull request

My proposed final tweaks before releasing v2.0

Please confirm that this pull request has done the following:

  • Tests added
  • Documentation added (where applicable)
  • Example added (either to an existing notebook or as a new notebook, where applicable) (N/A)
  • Description in CHANGELOG.rst added

Adding to CHANGELOG.rst

Please add a single line in the changelog notes similar to one of the following:

- (`#XX <http://link-to-pr.com>`_) Added feature which does something
- (`#XX <http://link-to-pr.com>`_) Fixed bug identified in (`#XX <http://link-to-issue.com>`_)

@znicholls znicholls marked this pull request as draft January 17, 2021 22:25
@codecov
Copy link

codecov bot commented Jan 17, 2021 •
edited
Loading

Codecov Report

Merging #323 (c3a1ab8) into master (c77443a) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #323      +/-   ##
==========================================
+ Coverage   93.34%   93.35%   +0.01%     
==========================================
  Files          18       18              
  Lines        2508     2513       +5     
  Branches      492      493       +1     
==========================================
+ Hits         2341     2346       +5     
  Misses        105      105              
  Partials       62       62
Impacted Files Coverage Δ
pymagicc/io/base.py 94.87% <100.00%> (+0.06%) ⬆️
pymagicc/io/prn_files.py 90.41% <100.00%> (ø)
pymagicc/scenarios/__init__.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e923c02...c3a1ab8. Read the comment docs.

@znicholls znicholls requested a review from rgieseke January 17, 2021 22:37
@znicholls
Copy link
Collaborator Author

@rgieseke I've pinned cftime temporarily to get the CI passing. I think it will work again once the windows wheel for cftime 1.3.1 is released, it just hasn't happened yet.

@rgieseke
Copy link
Member

Examples in the Readme need to be converted ...

@znicholls
Copy link
Collaborator Author

znicholls commented Jan 18, 2021 •
edited
Loading

Examples in the Readme need to be converted

Do we want to keep them? If so, we should probably work out how to test them (#258...)

@rgieseke
Copy link
Member

I'm fine with removing them (or some of them).

Tools could be

https://byexamples.github.io/byexample/

https://github.com/nschloe/exdown

@znicholls
Copy link
Collaborator Author

https://github.com/nschloe/exdown

beautiful! Will have a play today

@znicholls
Copy link
Collaborator Author

it just hasn't happened yet

Issue here, not sure how quickly it will happen... Unidata/cftime#224

@znicholls
Copy link
Collaborator Author

Issue here, not sure how quickly it will happen... Unidata/cftime#224

It appears to be a Python 3.6 issue, do we drop support?

@rgieseke
Copy link
Member

Issue here, not sure how quickly it will happen... Unidata/cftime#224

It appears to be a Python 3.6 issue, do we drop support?

"Following the release of 3.6.8, we plan to provide security fixes for Python 3.6 as needed through 2021, five years following its initial release."
https://www.python.org/downloads/release/python-360/

Not sure how widespread 3.6 is on Linux distros as default or macOS (or in the OpenSCM community ...).
Droppping the tests on Windows for 3.6 is probably fine.

@lewisjared
Copy link
Collaborator

We can pin cftime to 1.3.0 for the time being. I wouldn't drop support Python 3.6 yet as we don't explicitly need any 3.7+ features, but happy to drop Windows Python3.6 tests

@rgieseke
Copy link
Member

Not sure how these Windows wheels are produced, but it looks like the split is from 1.3.0 to 1.3.1

https://www.lfd.uci.edu/~gohlke/pythonlibs/#cftime

Maybe easiest to just drop the test on Windows ...

@znicholls
Copy link
Collaborator Author

Maybe easiest to just drop the test on Windows

Fixed it I think

@znicholls znicholls marked this pull request as ready for review January 19, 2021 12:21
lewisjared
lewisjared approved these changes Jan 19, 2021
View reviewed changes
Copy link
Collaborator

@lewisjared lewisjared left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brilliant! Good pickup on the rcps

@znicholls znicholls merged commit d3eda05 into master Jan 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lewisjared lewisjared lewisjared approved these changes

@rgieseke rgieseke rgieseke approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@znicholls @rgieseke @lewisjared